feat(connect): add initial support for connect component #2280
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2280 +/- ##
==========================================
+ Coverage 51.01% 51.18% +0.16%
==========================================
Files 104 104
Lines 2025 2032 +7
Branches 599 601 +2
==========================================
+ Hits 1033 1040 +7
Misses 889 889
Partials 103 103
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| } | ||
|
|
||
| try { | ||
| const connect = await window.braintree.connect.create({ |
There was a problem hiding this comment.
I remember one issue where I was unable to rely on window.braintree because the npm module (if npm-installed) did not attach itself to the window object - just mentioning it in case the integration pattern should support npm install braintree-web
There was a problem hiding this comment.
We're ok here because the AXO module we load above is what will append that window object.
src/connect/component.jsx
Outdated
| getLogger, | ||
| } from "@paypal/sdk-client/src"; | ||
|
|
||
| const sendCountMetric = ({ |
There was a problem hiding this comment.
I'm totally going to copy this for a different component! <- would it make sense to put sendCountMetric in a shared util file somewhere?
There was a problem hiding this comment.
@jshawl yes! @siddy2181 is actually working on that and moving it to sdk-client. It's used in SPB a bunch, and we want it here. We copied it over as we navigate the timeline dance, but the intent is to use a shared util of sendCountMetric from sdk-client.
| }); | ||
| } catch (error) { | ||
| sendCountMetric({ | ||
| name: "pp.app.paypal_sdk.connect.init.error.count", |
There was a problem hiding this comment.
should this be pp.app.paypal_sdk.connect.load.error.count ?
There was a problem hiding this comment.
We are keeping all errors under the same event name. Which then will be funneled based on loadvs create based on the event dimension param
| }); | ||
|
|
||
| sendCountMetric({ | ||
| name: "pp.app.paypal_sdk.connect.init.success.count", |
There was a problem hiding this comment.
did you follow a guideline for this name? (I'm about to do the same with another component but want to standardize with what you've done here!)
There was a problem hiding this comment.
@jshawl we referenced the card-fields stuff we'd done before but otherwise no we didn't know of a guide. Does one exist?
| try { | ||
| loadResult = await loadAxo({ | ||
| platform: "PPCP", | ||
| btSdkVersion: "3.97.3-connect-alpha.6.1", |
There was a problem hiding this comment.
So far this has to stay hardcoded. More to come, though.
Co-authored-by: elizabethmv <[email protected]>
| errorName: "connect_init_error", | ||
| }, | ||
| }); | ||
| throw new Error(error); |
There was a problem hiding this comment.
we should also log these errors log.error() so that we can investigate them in the future. I don't think we need logs for anything else, the metrics will be enough for the init and success cases. We'd want to see the errors themselves in cal though if theres an issue
dist/button.js
Outdated
There was a problem hiding this comment.
Ugh whyyy. I'm going to remove these, I'm not ok with incorporating other changes like this for this scenario.
| const cmid = getClientMetadataID(); | ||
| const clientID = getClientID(); |
There was a problem hiding this comment.
Should these be camel case? I don't know anymore when it comes to "id".
| const cmid = getClientMetadataID(); | |
| const clientID = getClientID(); | |
| const cmId = getClientMetadataID(); | |
| const clientId = getClientID(); |
Description
The initial component for Connect to be available on the PPCP SDK. Still in development and gated at the Connect Portal layer.
Why are we making these changes? Include references to any related Jira tasks or GitHub Issues
Ticket DTPPCPSDK-1367 for a variety of work.
Dependent Changes (if applicable)
Groups who should review (if applicable)
❤️ Thank you!